Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Overhaul Github Actions #2040

Merged
merged 38 commits into from
May 13, 2024
Merged

feat: Overhaul Github Actions #2040

merged 38 commits into from
May 13, 2024

Conversation

ajnavarro
Copy link
Contributor

@ajnavarro ajnavarro commented May 6, 2024

Github Actions refactoring

This PR aims to heavily simplify our CI and the way we are running our tests. To remove as much cognitive load as possible, Go default test commands are used for testing (a.k.a go test ./...) instead of intricated makefiles.

This PR is the first of several PRs cleaning and simplifying the test suite.

On this PR:

  • We are simplifying all the CI workflows using reusable workflows on all existing Go modules living on the monorepo.
  • Improvements made on contribs on chore: make contribs/ ci and makefiles dynamic #2013 are applied on this PR (CC @moul) (closes chore: make contribs/ ci and makefiles dynamic #2013)
  • Codecov tasks are also simplified, creating a tag per module, making codecov happier when uploading the reports and the code coverage report more understandable and easy to follow. Old tags must be deleted after merging this PR.
  • Now all tests are executed. The longest test suite is gnovm (6 mins). I don't think it is worth it to add the -short tag taking into account the extra work that it causes when tests are failing only in master because we are not running them on all the PRs. Apart from that, tests that should have been executed on CI are executed now.
  • Docker images are only built on master branch.

TODO:

These are some of the missing improvements on this PR that will be addressed in following ones:

  • Remove custom docker builds and use goreleaser
  • Check examples workflow and see how we can better test them maybe using go tests.
  • Check makefiles and simplify them
  • Should we move portal loop stuff outside the repo?
  • Test with several go versions when needed
  • Add cache where it makes sense
  • Fix race conditions and run tests with -race flag
  • Rebump benchmarks if possible
  • Build binaries and serve them as artifacts on every PR
  • TestPackages should only test the standard libraries. Rename it to TestStdlibs
  • Move failing challenges to issues

Extra checks:

  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label May 6, 2024
@moul
Copy link
Member

moul commented May 6, 2024

Related with #2013

@github-actions github-actions bot added the 📦 🌐 tendermint v2 Issues or PRs tm2 related label May 6, 2024
gnovm/tests/package_test.go Show resolved Hide resolved
gnovm/tests/file_test.go Show resolved Hide resolved
@thehowl
Copy link
Member

thehowl commented May 6, 2024

Overall, you seem to be also making a work to ensure that the "standard scenario" for testing is one where doing "go test ./..." just works. I can agree with that; I'd like to see the exceptions we have on the Makefiles (and copied into the CI) reduced as well.

However, consider that aside from the -run flags in gnovm/Makefile exclude some tests (like challenges, and the packages), consider also that we always pass the -short flag when testing the tests/files directory because some files (the ones with _long.gno) take a huge amount of time.

They are currently not covered; if you want to test them by adding a daily CI (rather than one at each commit) testing for them then go for it. But the CI should still run these with -short.

Copy link

codecov bot commented May 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 21.79%. Comparing base (977a3f4) to head (29bd559).

Files Patch % Lines
misc/autocounterd/cmd/cmd_start.go 0.00% 6 Missing ⚠️
misc/loop/cmd/snapshotter.go 0.00% 3 Missing ⚠️
misc/loop/cmd/cmd_serve.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2040       +/-   ##
===========================================
- Coverage   55.01%   21.79%   -33.22%     
===========================================
  Files         481       24      -457     
  Lines       67432     2092    -65340     
===========================================
- Hits        37097      456    -36641     
+ Misses      27318     1619    -25699     
+ Partials     3017       17     -3000     
Flag Coverage Δ
contribs/gnofaucet 14.46% <ø> (?)
contribs/gnomd 0.00% <ø> (?)
gno.land-_test.gnokey ?
gno.land-_test.gnoland ?
gno.land-_test.pkgs ?
gnovm-_test.cmd ?
gnovm-_test.gnolang.native ?
gnovm-_test.gnolang.other ?
gnovm-_test.gnolang.pkg0 ?
gnovm-_test.gnolang.pkg1 ?
gnovm-_test.gnolang.pkg2 ?
gnovm-_test.gnolang.realm ?
gnovm-_test.gnolang.stdlibs ?
gnovm-_test.pkg ?
go-1.21.x ?
go-1.22.x ?
misc ?
misc-_test.genstd ?
misc/autocounterd 0.00% <0.00%> (?)
misc/genproto 0.00% <ø> (?)
misc/genstd 73.90% <ø> (?)
misc/goscan 0.00% <ø> (?)
misc/logos 17.38% <ø> (?)
misc/loop 0.00% <0.00%> (?)
tm2-_test.flappy ?
tm2-_test.pkg.amino ?
tm2-_test.pkg.bft ?
tm2-_test.pkg.others ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ajnavarro ajnavarro force-pushed the improv/overhaul-ci branch 4 times, most recently from 1213b51 to 4a11406 Compare May 8, 2024 09:22
@ajnavarro ajnavarro marked this pull request as ready for review May 8, 2024 09:38
@ajnavarro ajnavarro requested review from moul, a team, jaekwon, piux2, deelawn and mvertes as code owners May 8, 2024 09:38
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be crazy, but the CI on this PR is significantly faster and snappier 👀

Let's go 🚀

.github/workflows/lint_template.yml Show resolved Hide resolved
.github/workflows/misc.yml Show resolved Hide resolved
.github/workflows/test_template.yml Show resolved Hide resolved
gnovm/pkg/gnolang/gno_test.go Show resolved Hide resolved
gnovm/tests/file_test.go Show resolved Hide resolved
gnovm/tests/package_test.go Outdated Show resolved Hide resolved
gnovm/tests/package_test.go Show resolved Hide resolved
misc/loop/cmd/cmd_serve.go Show resolved Hide resolved
@zivkovicmilos
Copy link
Member

@thehowl did you get a chance to recheck this? 👀

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM. a few comments; on the ones from the previous review, please act on them and we should be good to go

.github/examples.yml Outdated Show resolved Hide resolved
.github/workflows/DELETEdocker.yml Outdated Show resolved Hide resolved
.github/workflows/build_template.yml Outdated Show resolved Hide resolved
.github/workflows/main_template.yml Show resolved Hide resolved
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
Signed-off-by: Antonio Navarro Perez <[email protected]>
@ajnavarro ajnavarro force-pushed the improv/overhaul-ci branch 3 times, most recently from 29bd559 to d4ad4da Compare May 13, 2024 14:32
@ajnavarro ajnavarro merged commit d7c04be into master May 13, 2024
186 of 187 checks passed
@ajnavarro ajnavarro deleted the improv/overhaul-ci branch May 13, 2024 16:12
DIGIX666 pushed a commit to kazai777/gno that referenced this pull request May 15, 2024
thehowl added a commit that referenced this pull request May 27, 2024
omarsy pushed a commit to TERITORI/gno that referenced this pull request Jun 3, 2024
moul added a commit that referenced this pull request Jul 4, 2024
Continues #2013

> People should focus on their contributions in the `contribs/MYFOLDER`
without updating a shared `Makefile` + `.github/contribs/*.yml`.

The CI part has been completed in #2040.
This pull request handles the Makefile portion.

---------

Signed-off-by: moul <[email protected]>
gfanton pushed a commit to gfanton/gno that referenced this pull request Jul 23, 2024
Continues gnolang#2013

> People should focus on their contributions in the `contribs/MYFOLDER`
without updating a shared `Makefile` + `.github/contribs/*.yml`.

The CI part has been completed in gnolang#2040.
This pull request handles the Makefile portion.

---------

Signed-off-by: moul <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants